-
Notifications
You must be signed in to change notification settings - Fork 2
feat: adds otel-android click instrumentation #200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…strumentation-take2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unmodified vendored file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unmodified vendored file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still investigating compilation issue related to bytebuddy and exclusion of okhttp-jvm dependency.
| private var timeoutStartNanos: Long = 0 | ||
|
|
||
| @Volatile | ||
| private var state = State.FOREGROUND |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since line 69 performs a non atomic operation (check and set), I think @Volatile here is not enough and we should replace it by wrapping the state in an AtomicReference to ensure another thread cannot change the state between the check and the update.
|
@tanderson-ld is this PR no longer needed? |
|
I can close it for now, but the branch is useful. |
Summary
O11Y-417
How did you test this change?
Bench testing at the moment. There is a story in the epic to add e2e tests.